[PM-33981] feat: Add device management UI and Account Security integration#2871
Open
andrebispo5 wants to merge 8 commits into
Open
[PM-33981] feat: Add device management UI and Account Security integration#2871andrebispo5 wants to merge 8 commits into
andrebispo5 wants to merge 8 commits into
Conversation
…ation - Add PillBadgeStyle.info (blue: #EEF6FF bg / #BEDBFF border / #1A41AC text, dark: #162455 bg / #1A41AC border / #DBE5F6 text) for the Current Session badge in DeviceRow - Add manageDevices feature flag; gate the Devices row in Account Security behind it; reorder Other section: Fingerprint → Two-step → Devices → Lock now → Log out → Delete account - Add DeviceManagementView (list with pull-to-refresh), DeviceRow (pill badge at top, Trusted label, Recently active and First login rows, ContentBlock-managed dividers and background, full-row tap via Button), and DeviceManagementProcessor (parallel loading, calendar-day activity status, pendingRequestMatchKey-based request matching) - Wire .deviceManagement route in SettingsRoute and SettingsCoordinator - Add DeviceManagementProcessorTests (Swift Testing) and DeviceManagementViewTests (ViewInspector) - Add AccountSecurityProcessor tests for manageDevicesTapped navigation and isManageDevicesEnabled feature-flag load
github-actions
Bot
added
app:password-manager
andrebispo5
added
ai-review
Contributor
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the device management UI layer built on top of the Core models: the new No blocking or actionable findings. The notable concerns raised earlier — platform-based pending-request matching (deliberately reverted from Code Review DetailsNo findings at or above the confidence threshold. Observations considered and intentionally not filed as findings:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the Device Management UI flow (gated behind a feature flag) and integrates it into Account Security/Settings navigation, including a new “info” pill badge style for highlighting the current session.
Changes:
- Introduces the Device Management screen (view + row UI) and a processor that loads devices + pending login requests in parallel and matches requests to devices.
- Wires a new
.deviceManagementroute through Settings and adds a feature-flagged “Devices” entry in Account Security. - Adds UI assets/localizations for the new screen and badge style, plus new unit/UI tests.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| BitwardenShared/UI/Platform/Settings/SettingsRoute.swift | Adds a new route case for device management navigation. |
| BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift | Wires the new route and presents DeviceManagementView; adds required services. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceRow.swift | New device list row UI (badges, trusted label, activity/login info). |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementView+ViewInspectorTests.swift | New ViewInspector UI tests for the Device Management view. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementView.swift | New screen: loading/empty/list states, refresh, toast, and navigation chrome. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementState.swift | Adds state for device loading and toast presentation. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementProcessorTests.swift | Adds processor tests for load/sort/matching behavior and navigation. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementProcessor.swift | Implements parallel loading, pending-request matching, sorting, navigation, and toast behavior. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementEffect.swift | Adds the effect(s) used by the new processor (loadData). |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/DeviceManagement/DeviceManagementAction.swift | Adds actions for device tap/dismiss/toast state updates. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityView.swift | Adds a feature-flagged “Devices” row and hides the old pending-requests section when enabled. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityState.swift | Adds isManageDevicesEnabled flag to drive UI gating. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityProcessorTests.swift | Adds coverage for the new navigation action and feature flag loading. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityProcessor.swift | Loads the manage-devices feature flag and navigates to device management on tap. |
| BitwardenShared/UI/Platform/Settings/Settings/AccountSecurity/AccountSecurityAction.swift | Adds .manageDevicesTapped action. |
| BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift | Adds FeatureFlag.manageDevices. |
| BitwardenResources/Localizations/en.lproj/Localizable.strings | Adds strings for device management UI labels/badges. |
| BitwardenResources/Colors.xcassets/Badge/badgeInfoText.colorset/Contents.json | Adds color asset for info badge text. |
| BitwardenResources/Colors.xcassets/Badge/badgeInfoBorder.colorset/Contents.json | Adds color asset for info badge border. |
| BitwardenResources/Colors.xcassets/Badge/badgeInfoBackground.colorset/Contents.json | Adds color asset for info badge background. |
| BitwardenKit/UI/Platform/Application/Views/PillBadgeView.swift | Adds .info badge style using the new color assets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…l badges Info border dark: #BEDBFF → #1A41AC Warning background dark: #FFF8F1 → #441600 Warning border dark: #FCD9BD → #8A2203 Warning text dark: #B23300 → #FCD9BD
Success background dark: #F0FDF4 → #032E15 Success border dark: #B9F8CF → #016630 Success text dark: #08540F → #B9F8CF Danger background dark: #FEF2F2 → #460809 Danger border dark: #FFC9C9 → #9F0712 Danger text dark: #951B2A → #FFC9C9
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## pm-33981/device-table-core #2871 +/- ##
==============================================================
+ Coverage 79.00% 79.03% +0.03%
==============================================================
Files 1157 1160 +3
Lines 73641 73850 +209
==============================================================
+ Hits 58183 58371 +188
- Misses 15458 15479 +21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- DeviceRow: only wrap in Button when hasPendingRequest; non-actionable rows are now plain non-interactive views with no button trait - FeatureFlag: add manageDevices to allCases (alphabetically between fillAssistTargetingRules and migrateMyVaultToMyItems) - DeviceManagementView+ViewInspectorTests: add import ViewInspector
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+161
to
+170
| if let index = updatedDevices.firstIndex(where: { device in | ||
| !device.deviceType.pendingRequestMatchKey.isEmpty && | ||
| device.deviceType.pendingRequestMatchKey.lowercased() | ||
| == request.requestDeviceType.lowercased() | ||
| }) { | ||
| // Only set if no pending request has been set yet (keeps the most recent). | ||
| if updatedDevices[index].pendingRequest == nil { | ||
| updatedDevices[index].pendingRequest = request | ||
| } | ||
| } |
…uestDeviceType for browser and extension
The server sets requestDeviceType to the platform display name ('Chrome',
'Safari', etc.) for all device variants including extensions. The
pendingRequestMatchKey property returned 'Chrome Extension' for
chromeExtension, which never matched the server-sent 'Chrome', causing
extension devices to never receive a pending request badge.
Revert to matching on platform directly; when a user has both a Chrome
browser and a Chrome extension active the match is best-effort (first
in the list wins), documented in an inline comment.
Comment on lines
+4
to
+10
| "color" : { | ||
| "color-space" : "srgb", | ||
| "components" : { | ||
| "alpha" : "1.000", | ||
| "blue" : "0xF2", | ||
| "green" : "0xF2", | ||
| "red" : "0xFE" | ||
| "color": { | ||
| "color-space": "srgb", | ||
| "components": { | ||
| "alpha": "1.000", | ||
| "blue": "0xF2", | ||
| "green": "0xF2", | ||
| "red": "0xFE" |
Collaborator
There was a problem hiding this comment.
❓ Any idea why this format changed? Is this Xcode changing this?
Contributor
Author
There was a problem hiding this comment.
I've normalized the spacing back to match the repo convention across all the affected files (danger + success) so the diff only shows the actual color value changes.
Comment on lines
+49
to
+59
| VStack(spacing: 20) { | ||
| Image(decorative: Asset.Images.Illustrations.devices) | ||
| .resizable() | ||
| .frame(width: 100, height: 100) | ||
|
|
||
| Text(Localizations.noDevicesFound) | ||
| .styleGuide(.body) | ||
| .foregroundStyle(SharedAsset.Colors.textPrimary.swiftUIColor) | ||
| .multilineTextAlignment(.center) | ||
| } | ||
| .frame(maxWidth: .infinity) |
Collaborator
There was a problem hiding this comment.
🎨 Could we use IllustratedMessageView for displaying the empty state to match other views?
| /// - Parameter devices: The devices to display. | ||
| /// | ||
| private func devicesList(_ devices: [DeviceListItem]) -> some View { | ||
| VStack(spacing: 24) { |
Collaborator
There was a problem hiding this comment.
❓ Is the VStack here necessary given that it has only one child (ContentBlock)?
4 tasks
Skip devices that already have a pending request assigned when matching login requests to devices, so multiple requests for the same platform (e.g. browser/extension variants that both map to "Chrome") can match distinct devices instead of the first match being repeatedly rejected. Use IllustratedMessageView for the empty state to match other views, and drop an unnecessary single-child VStack. Normalize colon spacing in the badge danger/success colorset JSON files back to the repo convention — Xcode's asset editor had reformatted them, obscuring the actual dark-mode color-value diff.
Comment on lines
+163
to
+167
| device.pendingRequest == nil && | ||
| !device.deviceType.platform.isEmpty && | ||
| device.deviceType.platform.lowercased() | ||
| == request.requestDeviceType.lowercased() | ||
| }) { |
Comment on lines
+159
to
+162
| // Match by platform name, skipping devices that already have a pending request | ||
| // assigned so that multiple requests for the same platform (e.g. browser and | ||
| // extension variants that both map to "Chrome") can match distinct devices. | ||
| if let index = updatedDevices.firstIndex(where: { device in |
Comment on lines
+1177
to
+1180
| "RecentlyActiveLabel" = "Recently active:"; | ||
| "RecentlyActiveX" = "Recently active: %1$@"; | ||
| "FirstLoginLabel" = "First login:"; | ||
| "FirstLoginX" = "First login: %1$@"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-33981
📔 Objective
⛓️ Depends on #2870 (PM-33981 Core models and API layer) — merge that PR first.
Adds the complete device management UI on top of the Core layer from the preceding PR.
PillBadgeStyle.info(blue palette) toPillBadgeViewfor the Current Session badgeDeviceManagementView(list with pull-to-refresh and empty state),DeviceRow(pill badge at top, Trusted label, Recently active and First login info rows, full-row tap), andDeviceManagementProcessor(parallel device + pending-request loading, calendar-day activity buckets,pendingRequestMatchKey-based request matching).deviceManagementroute inSettingsRouteandSettingsCoordinatormanageDevicesfeature flag; gates a "Devices" row in the Account Security Other section behind it; reorders the section: Account fingerprint phrase → Two-step login → Devices → Lock now → Log out → Delete accountDeviceManagementProcessorTests(Swift Testing) andDeviceManagementViewTests(ViewInspector); addsAccountSecurityProcessorTestscoverage for the new action and flag load📸 Screenshots